Skip to content

quic: cleaning up redundant tests#16660

Merged
yanavlasov merged 1 commit intoenvoyproxy:mainfrom
alyssawilk:cleanup
May 26, 2021
Merged

quic: cleaning up redundant tests#16660
yanavlasov merged 1 commit intoenvoyproxy:mainfrom
alyssawilk:cleanup

Conversation

@alyssawilk
Copy link
Copy Markdown
Contributor

the only ones I was unsure of were UpstreamReadDisabledOnGiantResponseBody vs DownstreamReadDisabledOnGiantPost vs LargeFlowControlOnAndGiantBody
it looks like small request / large response vs large request / small response vs large request / large response.
protocol integration test has large request / large response which I think is sufficient but lmk if I'm missing something.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Copy Markdown
Contributor

@danzh2010 danzh2010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning up!


auto response = sendRequestAndWaitForResponse(default_request_headers_, 0, response_headers, 0);
auto response = sendRequestAndWaitForResponse(request_headers, 0, response_headers, 0);
if (downstreamProtocol() == Http::CodecClient::Type::HTTP3) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to check envoy.reloadable_features.header_map_correctly_coalesce_cookies?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given there was no CI which tested with it flipped I think that's fine

@danzh2010
Copy link
Copy Markdown
Contributor

the only ones I was unsure of were UpstreamReadDisabledOnGiantResponseBody vs DownstreamReadDisabledOnGiantPost vs LargeFlowControlOnAndGiantBody
it looks like small request / large response vs large request / small response vs large request / large response.
protocol integration test has large request / large response which I think is sufficient but lmk if I'm missing something.

They were added to distinguish downstream and upstream flow control performance problems more easily, i.e. flow control window. They have the same coverage as those in protocol integration tests, made debugging easier.

@yanavlasov yanavlasov merged commit 694df35 into envoyproxy:main May 26, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk deleted the cleanup branch August 4, 2022 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants